fix: Loop node deletion without deleting loop body#4090
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| } | ||
| props.nodeModel.graphModel.deleteNode(props.nodeModel.id) | ||
| }) | ||
| props.nodeModel.graphModel.eventCenter.emit('delete_node') |
There was a problem hiding this comment.
There are a few adjustments to ensure the code is more efficient and readable:
const deleteNode = () => {
confirmAlert({
title: t('common.confirm'),
message: t('workflow.remove_node_confirm.message'), // Assuming you have a message string
confirmButtonText: t('common.confirm'),
confirmButtonClass: 'danger',
}).then(() => {
const nodeModel = props.nodeModel;
if (!nodeModel || !(nodeModel.type && nodeModel.graphModel)) {
console.error("Node model or its graph model is not properly set.");
return;
}
if (nodeModel.type !== WorkflowType.LoopNode) {
nodeModel.graphModel.deleteNode(nodeModel.id);
props.nodeModel.graphModel.eventCenter.emit('delete_node');
return;
}
const { id } = nodeModel,
outNodes = props.nodeModel.graphModel.getAllOutgoingNodesById(id),
loopBodyNodeId = outNodes.find(no => no.nodeType === "loop-body-node");
if (loopBodyNodeId) {
nodeModel.graphModel.deleteNode(loopBodyNodeId);
}
nodeModel.graphModel.deleteNode(id);
props.nodeModel.graphModel.eventCenter.emit('delete_node');
});
};Potential Improvements:
- Error Handling: Added checks to see if
nodeModelexists and has bothtypeandgraphModel. - Message String: Assumed there's a translation function for displaying confirmation messages. Replace
'workflow.remove_node_confirm.message'with your actual message key. - Simplified Loop Body Deletion Logic: Used ES6 syntax (
...) for better readability, but this was initially suggested mistakenly.
This version improves the robustness of error handling and makes the logic clearer without changing functionality significantly.
1 participant
fix: Loop node deletion without deleting loop body